OCPBUGS-63219: Remove NLB hairpin workaround from dual-stack test - #31453
OCPBUGS-63219: Remove NLB hairpin workaround from dual-stack test#31453gcs278 wants to merge 1 commit into
Conversation
The manual target-group-attributes annotation to disable client IP preservation is no longer needed. New NLB IngressControllers default to PROXY protocol (openshift/cluster-ingress-operator#1426), which fixes hairpin connection failures without manual intervention. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: automatic mode |
|
Skipping CI for Draft Pull Request. |
|
@gcs278: This pull request references Jira Issue OCPBUGS-63219, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
WalkthroughThe dual-stack NLB ingress-controller test removes the annotation that disabled target group client IP preservation. The remaining labeling, route, backend, DNS, and curl verification steps are unchanged. ChangesDual-stack NLB test
Estimated code review effort: 1 (Trivial) | ~2 minutes 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/test ? |
|
/payload-job-with-prs periodic-ci-openshift-release-main-nightly-5.0-e2e-metal-ipi-ovn-dualstack openshift/cluster-ingress-operator#1426 |
|
@gcs278: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/0b3cbda0-8ba9-11f1-920c-8c9cd0cf844f-0 |
|
/pipeline required |
|
Scheduling required tests: |
|
/retest |
|
unrelated |
1 similar comment
|
unrelated |
|
Good job on the PROXY proto for NLB! /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gcs278, jcmoraisjr The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/test ? |
|
@CodeRabbit what tests should we run to verify this dualstack e2e jobs still works that's being changed in this PR? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Test file outline =='
ast-grep outline test/extended/router/dualstack.go --items all || true
echo '== NLB and target-group references in the test file =='
rg -n -C 8 'NLB|Network Load Balancer|target-group|preserve_client_ip|PROXY|dual.?stack' test/extended/router/dualstack.go || true
echo '== Repository references to the affected test and requested CI job =='
rg -n -i -C 3 'dualstack.*nlb|nlb.*dualstack|e2e-metal-ipi-ovn-dualstack|aws-load-balancer-target-group-attributes|preserve_client_ip' . \
-g '!vendor/**' -g '!**/.git/**' || true
echo '== Candidate CI configuration files =='
fd -HI -t f '(config|jobs|release|periodic).*\.ya?ml$|.*\.ya?ml$' . \
| xargs -r rg -l -i 'e2e-metal-ipi-ovn-dualstack|dualstack' \
| head -50Length of output: 7832 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== Complete affected NLB spec =='
sed -n '52,114p' test/extended/router/dualstack.go
echo '== Extended-test invocation and Ginkgo filtering documentation =='
rg -n -i -C 4 \
'ginkgo\.(focus|skip)|--ginkgo\.(focus|skip)|--run-?regex|run.*extended|extended.*test' \
README.md docs test hack openshift-hack 2>/dev/null || true
echo '== Test commands and suite definitions that include router tests =='
rg -n -i -C 4 \
'test/extended|extended.*router|router.*extended|run-test' \
Makefile* hack test openshift-hack 2>/dev/null || trueLength of output: 201113
Run it with Verify that this spec passes in the job output: The spec creates an AWS NLB router shard. It then verifies route DNS resolution and route access with both IPv4 and IPv6. This covers the former hairpin failure path. No additional unit test is needed for this change. A unit test cannot validate AWS NLB provisioning or the end-to-end IPv4/IPv6 connection path. You are interacting with an AI system. |
|
@coderabbitai[bot]: user coderabbitai[bot] is not trusted for pull request #31453 |
|
We should definitely run the dualstack tests to verify before merging: |
|
/test e2e-metal-ipi-ovn-bgp-virt-dualstack |
|
/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-installer-dualstack-ipv6-primary-techpreview |
|
@gcs278: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/4fe7b7f0-8d1d-11f1-98c7-43e19aae3884-0 |
|
/payload-job periodic-ci-openshift-release-main-nightly-5.0-e2e-aws-ovn-installer-dualstack-ipv4-primary-techpreview |
|
@gcs278: trigger 1 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/5d346980-8d1d-11f1-90bb-3970c871a3a9-0 |
|
I forgot to realize this test is an AWS-only test. Kicked off the two payload jobs that matter. I'll hold until i get a good CI result. |
|
@CodeRabbit can you the two payload jobs i created to make sure there are no regressions from the test I started: https://pr-payload-tests.ci.openshift.org/runs/ci/4fe7b7f0-8d1d-11f1-98c7-43e19aae3884-0 and https://pr-payload-tests.ci.openshift.org/runs/ci/5d346980-8d1d-11f1-90bb-3970c871a3a9-0 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
runs=(
"https://pr-payload-tests.ci.openshift.org/runs/ci/4fe7b7f0-8d1d-11f1-98c7-43e19aae3884-0"
"https://pr-payload-tests.ci.openshift.org/runs/ci/5d346980-8d1d-11f1-90bb-3970c871a3a9-0"
)
for run in "${runs[@]}"; do
echo "===== $run ====="
curl --fail --location --silent --show-error "$run" \
-o "/tmp/payload-run-$(basename "$run").html"
rg -n -i -C 3 \
'pass|fail|success|failure|pending|running|complete|job|prow|build|artifact|dual.?stack|router|ingress|NLB|Network Load Balancer' \
"/tmp/payload-run-$(basename "$run").html" \
| head -500 || true
echo
doneLength of output: 3416 🏁 Script executed: #!/bin/bash
set -euo pipefail
builds=(
"https://prow.ci.openshift.org/view/gs/test-platform-results/logs/openshift-origin-31453-nightly-5.0-e2e-aws-ovn-installer-dualstack-ipv6-primary-techpreview/2083286778822791168"
"https://prow.ci.openshift.org/view/gs/test-platform-results/logs/openshift-origin-31453-nightly-5.0-e2e-aws-ovn-installer-dualstack-ipv4-primary-techpreview/2083286873333043200"
)
for build in "${builds[@]}"; do
id="${build##*/}"
html="/tmp/prow-${id}.html"
echo "===== Prow build ${id} ====="
curl --fail --location --silent --show-error "$build" -o "$html"
echo "-- page status and links --"
rg -n -i -C 2 \
'failed|failure|error|artifact|build-log|finished\.json|junit|test.*result|e2e|dual.?stack|router|ingress' \
"$html" | head -400 || true
echo "-- candidate artifact URLs --"
rg -o 'https?://[^"< ]+' "$html" \
| sed 's/&/\&/g' \
| rg -i '(artifact|build-log|finished\.json|junit|test.*result|e2e)' \
| sort -u | head -100 || true
echo
doneLength of output: 15510 🏁 Script executed: #!/bin/bash
set -euo pipefail
base="https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/test-platform-results/logs"
jobs=(
"openshift-origin-31453-nightly-5.0-e2e-aws-ovn-installer-dualstack-ipv6-primary-techpreview/2083286778822791168|artifacts/e2e-aws-ovn-installer-dualstack-ipv6-primary-techpreview"
"openshift-origin-31453-nightly-5.0-e2e-aws-ovn-installer-dualstack-ipv4-primary-techpreview/2083286873333043200|artifacts/e2e-aws-ovn-installer-dualstack-ipv4-primary-techpreview"
)
for entry in "${jobs[@]}"; do
build="${entry%%|*}"
artifact_dir="${entry##*|}"
suffix="${build##*/}"
xml="/tmp/junit-${suffix}.xml"
finished="/tmp/finished-${suffix}.json"
echo "===== ${build} ====="
curl --fail --location --silent --show-error \
"${base}/${build}/${artifact_dir}/openshift-e2e-test/artifacts/junit/junit_e2e__"*"xml" \
-o "$xml" 2>/dev/null || true
# Use the exact filenames known from the Prow page when shell glob URLs are not supported.
if [[ ! -s "$xml" ]]; then
if [[ "$build" == *ipv6* ]]; then
junit_name="junit_e2e__20260731-213431.xml"
else
junit_name="junit_e2e__20260731-213737.xml"
fi
curl --fail --location --silent --show-error \
"${base}/${build}/${artifact_dir}/openshift-e2e-test/artifacts/junit/${junit_name}" \
-o "$xml"
fi
curl --fail --location --silent --show-error \
"${base}/${build}/${artifact_dir}/openshift-e2e-test/finished.json" \
-o "$finished" || true
echo "-- openshift-e2e-test step --"
cat "$finished" 2>/dev/null || true
echo
echo "-- affected router dual-stack spec --"
rg -ni -C 2 \
'should be reachable via IPv4 and IPv6 through a dual-stack ingress controller|dual-stack ingress controller|NLB|Network Load Balancer' \
"$xml" || true
echo "-- failed or errored test cases --"
python3 - "$xml" <<'PY'
import sys
import xml.etree.ElementTree as ET
root = ET.parse(sys.argv[1]).getroot()
failed = []
for case in root.iter("testcase"):
problems = [child for child in list(case) if child.tag in ("failure", "error")]
if problems:
name = case.attrib.get("name", "")
classname = case.attrib.get("classname", "")
message = " | ".join(
(p.attrib.get("message") or (p.text or "").strip().splitlines()[0:1][0] if (p.text or "").strip() else "")
for p in problems
)
failed.append((classname, name, message))
print(f"failure/error count: {len(failed)}")
for classname, name, message in failed[:100]:
print(f"- {classname}: {name}")
if message:
print(f" {message[:500]}")
PY
echo
doneLength of output: 3635 🏁 Script executed: #!/bin/bash
set -euo pipefail
bucket="https://storage.googleapis.com/test-platform-results/logs"
entries=(
"ipv6-primary|openshift-origin-31453-nightly-5.0-e2e-aws-ovn-installer-dualstack-ipv6-primary-techpreview/2083286778822791168|artifacts/e2e-aws-ovn-installer-dualstack-ipv6-primary-techpreview|junit_e2e__20260731-213431.xml"
"ipv4-primary|openshift-origin-31453-nightly-5.0-e2e-aws-ovn-installer-dualstack-ipv4-primary-techpreview/2083286873333043200|artifacts/e2e-aws-ovn-installer-dualstack-ipv4-primary-techpreview|junit_e2e__20260731-213737.xml"
)
for entry in "${entries[@]}"; do
IFS='|' read -r variant build artifact_dir junit_name <<<"$entry"
xml="/tmp/${variant}-junit.xml"
echo "===== ${variant} ====="
curl --fail --location --silent --show-error \
"${bucket}/${build}/${artifact_dir}/openshift-e2e-test/artifacts/junit/${junit_name}" \
-o "$xml"
echo "-- artifact type --"
file "$xml"
echo "-- changed router dual-stack spec --"
rg -ni -C 2 \
'should be reachable via IPv4 and IPv6 through a dual-stack ingress controller|dual-stack ingress controller|NLB|Network Load Balancer' \
"$xml" || true
echo "-- failed or errored test cases --"
python3 - "$xml" <<'PY'
import sys
import xml.etree.ElementTree as ET
root = ET.parse(sys.argv[1]).getroot()
failed = []
for case in root.iter("testcase"):
problems = [child for child in case if child.tag in ("failure", "error")]
if problems:
messages = []
for problem in problems:
text = (problem.attrib.get("message") or problem.text or "").strip()
messages.append(text.splitlines()[0] if text else "")
failed.append((
case.attrib.get("classname", ""),
case.attrib.get("name", ""),
" | ".join(messages),
))
print(f"failure/error count: {len(failed)}")
for classname, name, message in failed:
print(f"- {classname}: {name}")
if message:
print(f" {message[:500]}")
PY
echo
doneLength of output: 2393 🏁 Script executed: #!/bin/bash
set -euo pipefail
bucket="https://storage.googleapis.com/test-platform-results/logs"
entries=(
"ipv6-primary|openshift-origin-31453-nightly-5.0-e2e-aws-ovn-installer-dualstack-ipv6-primary-techpreview/2083286778822791168|artifacts/e2e-aws-ovn-installer-dualstack-ipv6-primary-techpreview|junit_e2e__20260731-213431.xml"
"ipv4-primary|openshift-origin-31453-nightly-5.0-e2e-aws-ovn-installer-dualstack-ipv4-primary-techpreview/2083286873333043200|artifacts/e2e-aws-ovn-installer-dualstack-ipv4-primary-techpreview|junit_e2e__20260731-213737.xml"
)
for entry in "${entries[@]}"; do
IFS='|' read -r variant build artifact_dir junit_name <<<"$entry"
xml="/tmp/${variant}-junit.xml"
echo "===== ${variant} ====="
curl --fail --location --silent --show-error \
"${bucket}/${build}/${artifact_dir}/openshift-e2e-test/artifacts/junit/${junit_name}" \
-o "$xml"
python3 - "$xml" <<'PY'
import sys
import xml.etree.ElementTree as ET
root = ET.parse(sys.argv[1]).getroot()
cases = list(root.iter("testcase"))
affected = [
c for c in cases
if any(term in (c.attrib.get("name", "") + " " + c.attrib.get("classname", "")).lower()
for term in ("dual-stack ingress controller", "network load balancer", "nlb"))
]
failed = []
for case in cases:
problems = [child for child in case if child.tag in ("failure", "error")]
if problems:
text = (problems[0].attrib.get("message") or problems[0].text or "").strip()
failed.append((
case.attrib.get("classname", ""),
case.attrib.get("name", ""),
text.splitlines()[0] if text else "",
))
print(f"testcase count: {len(cases)}")
print(f"affected-spec matches: {len(affected)}")
for case in affected:
status = "failed" if any(child.tag in ("failure", "error") for child in case) else "passed"
print(f"- {status}: {case.attrib.get('classname', '')}: {case.attrib.get('name', '')}")
print(f"failure/error count: {len(failed)}")
for classname, name, message in failed:
print(f"- {classname}: {name}")
if message:
print(f" {message[:500]}")
PY
echo
doneLength of output: 10404
The changed test passed in both jobs: Both jobs are marked failed overall, but neither JUnit report lists the changed dual-stack ingress-controller test as a failure. The IPv6-primary job has unrelated failures in router stress, router metrics, operator status, and You are interacting with an AI system. |
|
Payload jobs look good - ignore the metal dualstack failures (this PR is only for AWS dualstack). I think we can merge |
|
Payload jobs prove that this jobs passes. /verified by CI |
|
@gcs278: This PR has been marked as verified by DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
@gcs278: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
Scheduling required tests: |
Summary
Remove the manual
service.beta.kubernetes.io/aws-load-balancer-target-group-attributes=preserve_client_ip.enabled=falseannotation workaround from the dual-stack NLB e2e test.New NLB IngressControllers now default to PROXY protocol (openshift/cluster-ingress-operator#1426), which disables native client IP preservation and fixes hairpin connection failures without manual annotation.
Affected test:
[sig-network-edge][OCPFeatureGate:AWSDualStackInstall][Feature:Router] should be reachable via IPv4 and IPv6 through a dual-stack ingress controllerDependencies